Conversation
|
Codex review: needs real behavior proof before merge. Reviewed August 12, 2026, 8:59 AM ET / 12:59 UTC. ClawSweeper reviewWhat this changesThe PR feeds a Codex credit-limit used percentage into automatic menu-bar percentage and usage-bar layout tokens when the existing Codex projection selects its credits fallback. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 9 items remain Keep open: the requested credit-only fallback remains useful, but this draft currently replaces an available automatic rate-window metric when any Codex lane is exhausted, changes the usage bar the same way, is dirty against current main, and has only synthetic-test proof. Priority: P2 Review scores
Verification
How this fits togetherCodexBar turns Codex usage and credit snapshots into a provider projection, then supplies semantic session, weekly, and automatic values to the configurable macOS menu-bar layout. The renderer converts those values into percentages and a three-cell usage bar for the status item and editor preview. flowchart LR
A[Codex usage snapshot] --> B[Codex usage projection]
C[Credit-limit snapshot] --> B
B --> D[Automatic window selection]
B --> E[Credit fallback decision]
D --> F[Menu-bar layout data]
E --> F
F --> G[Percentage and usage-bar tokens]
G --> H[Status item and editor preview]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Rebase onto current main, use the credit value only when the automatic rate window is absent, cover both the credit-only and available-window cases, then attach redacted freshly built-app evidence for the intended credit-only display. Do we have a high-confidence way to reproduce the issue? Yes, at source level: an exhausted resetting Codex session plus a valid weekly lane and 3%-used credit limit reaches the fallback while an automatic rate window remains available. No after-fix real-app run has been supplied. Is this the best way to solve the issue? No. The fallback should fill an absent automatic rate window rather than override one; that preserves existing automatic-layout semantics while solving the stated credit-only case. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a90dfed5c264. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (15 earlier review cycles; latest 8 shown)
|
4756403 to
d0fc33a
Compare
|
Thanks @McBrosa. Closing as superseded by the implementation now on 7350a1e1cf92 added the monthly-credit projection, and 30a389d6e664 completed the case where the usage snapshot has not loaded. Both are reachable from current main. The earlier August 12 review predates these fixes. The current projection provides a monthly automatic meter for credit-only accounts, without inventing Session/Weekly windows or replacing available rate windows. Status-item render data and the live editor path use that automatic projection; Auto % and Usage bar honor used/remaining direction. This retains the later optional-credit visibility setting as well. Fresh verification: This closure is based on current source and executed regression tests, not a new real-account UI capture. Thank you for identifying the gap and preparing the original regression work. |
Summary
Root cause
The legacy status-item renderer already had a Codex credits fallback, but the stored menu-bar layout renderer only received
RateWindowvalues. Credit-only Business accounts therefore supplied valid credit-limit data to the menu card whileAuto %received no window and rendered the standard en-dash placeholder.This carries the existing
CodexConsumerProjection.menuBarFallbackdecision into layout render data as a used percentage. The renderer consumes it only for automatic usage tokens, leaving session and weekly quotas unavailable when those windows do not exist.Proof
swift test --filter 'MenuBarLayoutRendererTests|StatusItemAnimationCodexCreditsTests'— 16 tests passedmake check— locale, documentation, packaging, SwiftFormat, and strict SwiftLint checks passed with zero violationsmake test— 812 selections across 68 groups passedThe status-item regression uses synthetic credit-limit data matching the report (
1,193.6 / 42,750) and asserts that the stored default layout renders3%instead of–. No live provider probe, Keychain access, or bundle-level screenshot was used.